octave: Do not --enable-link-all-dependencies on Darwin #368376
Conversation
| # Add Octave's shared objects to ld library path for some packages which | ||
| # need to link their C/C++/Fortran code against Octave's internals. | ||
| LD_LIBRARY_PATH = lib.makeLibraryPath nativeBuildInputs'; | ||
|
|
There was a problem hiding this comment.
I'm a little confused about how LD_LIBRARY_PATH helps.
however, was able to get the build to work on darwin with this
diff --git a/pkgs/development/octave-modules/ltfat/default.nix b/pkgs/development/octave-modules/ltfat/default.nix
index 43fe020b322a..d22a78938b5a 100644
--- a/pkgs/development/octave-modules/ltfat/default.nix
+++ b/pkgs/development/octave-modules/ltfat/default.nix
@@ -9,6 +9,7 @@
lapack,
blas,
portaudio,
+ octave,
jdk,
}:
@@ -21,6 +22,8 @@ buildOctavePackage rec {
sha256 = "sha256-FMDZ8XFhLG7KDoUjtXvafekg6tSltwBaO0+//jMzJj4=";
};
+ NIX_LDFLAGS = "-L${lib.getLib octave}/lib/octave/9.3.0";
+
buildInputs = [
fftw
fftwSinglePrectho didn't test the plugin to see if it actually works -- there were some linker warnings.
it seems linux is building fine on master tho: https://hydra.nixos.org/build/282688853
[edit] on master octave on darwin needs #367594 to build
There was a problem hiding this comment.
looking at the darwin vs linux outputs with NIX_DEBUG=1 can see that linux doesn't link to -loctave or -octinterp. but sometimes darwin is fine. eg polyboolmex.mex seems to add -L/nix/store/svnxlwq9p3lpl3wjx70kx03qzf5zlhkw-octave-9.3.0/lib/octave/9.3.0 and builds but then comp_atrousfilterbank_td.oct comp_cellcoef2tf.oct comp_chirpzt.oct comp_col2diag.oct comp_dct.oct comp_dst.oct comp_dwilt.oct and comp_dwiltiii.oct don't and fail.
There was a problem hiding this comment.
ok looking at the bundle in oct/Makefile_unix it seems to export LDFLAGS
export LDFLAGS := -L$(shell $(MKOCTFILE) -p LIBDIR) $(FLIBS) $(LAPACK_LIBS) $(BLAS_LIBS) $(FFTW_LIBS)
and thru some experimentation when LDFLAGS are set I guess that overrides the flags used by mkoctfile because
make -f Makefile_unix comp_atrousfilterbank_td.oct
mkoctfile -strip -I../src/modules/libltfat/include -DNDEBUG -L../lib -lltfat comp_atrousfilterbank_td.cc
mkoctfile: stripping disabled on this platform
ld: library not found for -loctinterp
but just can comment out LDFLAGS in the makefile and then the command succeeds.
so this seems like an upstream issue.
There was a problem hiding this comment.
ok -- sorry for all the posts. I just realized that can use patches in the octave builder adding -L$(mkoctfile -p OCTLIBDIR) to LDFLAGS in the makefile is sufficient.
should file an issue: https://github.com/ltfat/ltfat/issues
don't clobber ldflags.diff
index 43fe020b322a..c5c48e0dc5e0 100644
--- a/pkgs/development/octave-modules/ltfat/default.nix
+++ b/pkgs/development/octave-modules/ltfat/default.nix
@@ -32,6 +32,11 @@ buildOctavePackage rec {
jdk
];
+ patches = [
+ # add OCTLIBDIR to ldflags
+ ./octlibdir-ldflags.diff
+ ];
+
meta = with lib; {
name = "The Large Time-Frequency Analysis Toolbox";
homepage = "https://octave.sourceforge.io/ltfat/index.html";
diff --git a/pkgs/development/octave-modules/ltfat/octlibdir-ldflags.diff b/pkgs/development/octave-modules/ltfat/octlibdir-ldflags.diff
new file mode 100644
index 000000000000..c73f5701b8a9
--- /dev/null
+++ b/pkgs/development/octave-modules/ltfat/octlibdir-ldflags.diff
@@ -0,0 +1,13 @@
+diff --git a/oct/Makefile_unix b/oct/Makefile_unix
+index 21e026b..c9d767a 100644
+--- a/oct/Makefile_unix
++++ b/oct/Makefile_unix
+@@ -36,7 +36,7 @@ endif
+ export CXXFLAGS := $(shell $(MKOCTFILE) -p CXXFLAGS) -std=gnu++11 -Wall -DLTFAT_LARGEARRAYS $(OPTCXXFLAGS)
+ # export is necessary, otherwise LFLAGS won't have any effect
+ # at least on Windows and on Mac
+-export LDFLAGS := -L$(shell $(MKOCTFILE) -p LIBDIR) $(FLIBS) $(LAPACK_LIBS) $(BLAS_LIBS) $(FFTW_LIBS)
++export LDFLAGS := -L$(shell $(MKOCTFILE) -p LIBDIR) -L$(shell $(MKOCTFILE) -p OCTLIBDIR) $(FLIBS) $(LAPACK_LIBS) $(BLAS_LIBS) $(FFTW_LIBS)
+ undefine LFLAGS
+ unexport LFLAGS
+
github-actions
bot
added
10.rebuild-linux: 0
There was a problem hiding this comment.
this change looks good but the title and commit message need to match -- perhaps:
octave: remove --enable-link-all-dependencies
darwin set --enable-link-all-dependencies which was the default for
homebrew and octave for a little bit but both reverted the change a
a couple of years ago:
https://github.com/Homebrew/homebrew-core/commit/a01651b19e4babace008b7dd94899807f1818116
https://github.com/gnu-octave/octave/commit/d4479bd8aef35911e07851ef3aee89ef3954604b
or something.
note: #368187 (comment) has a bunch of references to when --enable-link-all-dependencies was removed in home-brew and disabled by default on octave.
I built this on aarch64-darwin and enabled tests and results were identical to that of x64 linux.
[edit] NOTE: I'm building on darwin with this change #367594 which is required since #357259 broke arpack on darwin
|
@paparodeo, excellent! Yep! I'll update the necessary PR stuff soon (and rebase the |
|
Given that @doronbehar is the current maintainer of the Octave definition, I also want to see his response to this as well. |
It seems like upstream Octave removed the requirement that Darwin default to linking all of its dependencies[1]. Some packages (ltfat) cannot link against Octave for some reason. Removing this optinal configure flag fixes this issue. [1] gnu-octave/octave@d4479bd
KarlJoad
force-pushed
the
octavePackages.ltfat/fix-ld-error
branch
from
d788985 to
9e3db9e
Compare
KarlJoad
changed the title
Some Octave packages ship C/C++/Fortran code that needs to be compiled and linked against Octave's internal shared objects. Previously,
ldcould not find these packages because$LD_LIBRARY_PATHwas empty.Closes #368187.
nixpkgs-reviewmarked some new packages as broken (bim, fits, msh), but ltfat builds correctly.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.